-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add unbind-default-keys
config option
#2733
Conversation
Could you add some notes on using this to the docs? https://github.com/helix-editor/helix/tree/f37ffaa3a1c7754238029390db00148ec8105874/book/src |
No problem. If you'd like me to include a new subsection or larger blurb, or if there's a better place to provide this specific documentation, let me know |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about renaming the config option to unbind_default_keys
?
cd3dc9c
to
d0f2c2f
Compare
unbind_defaults
config optionunbind-default-keys
config option
86caa8b
to
cc60238
Compare
Is there any movement on this? I'd really like to try out this feature. |
cc60238
to
8ecb962
Compare
I updated the feature to the current version of helix and addressed the above changes. Now it only adds the config option |
ref: helix-editor/helix#2720 ref: helix-editor/helix#2733 Co-authored-by: Linden Krouse <[email protected]>
* Add `unbind_default_keys` config option Based on helix-editor#2733 * Fix lint issues
let mut keys; | ||
match config.unbind_default_keys { | ||
true => keys = HashMap::default(), | ||
false => keys = keymap::default(), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let mut keys; | |
match config.unbind_default_keys { | |
true => keys = HashMap::default(), | |
false => keys = keymap::default(), | |
} | |
let mut keys = match config.unbind_default_keys { | |
true => HashMap::default(), | |
false => keymap::default(), | |
}; |
let mut keys; | ||
match local.unbind_default_keys { | ||
true => keys = HashMap::default(), | ||
false => keys = keymap::default(), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let mut keys; | |
match local.unbind_default_keys { | |
true => keys = HashMap::default(), | |
false => keys = keymap::default(), | |
} | |
let mut keys = match local.unbind_default_keys { | |
true => HashMap::default(), | |
false => keymap::default(), | |
}; |
let keymap = match keymaps.get(&mode) { | ||
Some(keymap) => keymap, | ||
None => return KeymapResult::NotFound, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let keymap = match keymaps.get(&mode) { | |
Some(keymap) => keymap, | |
None => return KeymapResult::NotFound, | |
}; | |
let Some(keymap) = keymaps.get(&mode) else { return KeymapResult::NotFound; }; |
This is the style used elsewhere in the code.
#9384 (comment), #2720 (comment) - we want to solve this when we switch config away from TOML rather than adding a new config option now |
Closes #2720.
There were a few changes I made to reduce confusion with some function/method locations and naming:
helix_term::keymap::merge_keys
was moved tohelix_term::config::Config::merge_keys
as a method because it only operates on and changesConfig
, and was modified to take a set of bindings to merge with itself. New functions were also added to handle the new optional behavior.Config::load
now is the single source of truth for loading and merging configs, which was previously also handled inhelix_term::application::new
in a slightly different but mostly redundant manner.Default
is removed fromConfig
and replaced withConfig::new
andConfig::with_default_keys
.new
returns a Config with no bindings whilewith_default_keys
returns a Config with the default bindings (which is the same behavior as the olddefault
). This to prevent confusion between a default ruststruct
and helix's default keybindings as these terms were previously conflated.Config::keys
no longer loads default bindings through serde when no keys are found as this was always redundant.helix_term::keymap::Keymaps::get
no longer crashes when a mode isn't found. This will also help with future mode additions (likely through plugins down the line).Let me know if any of these changes should be handled differently
EDIT: After updating to a much newer version of helix, this PR only adds the new option
unbind_default_keys
which does not populate the initial keymap at all. This also includes the fix where Helix would crash if a keybind for a non-existent mode is triggered